-
Notifications
You must be signed in to change notification settings - Fork 3
Add SiteRepository with polymorphic site support #1000
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
603b982 to
8d970ad
Compare
Extend DbSite to support multiple site types (self-hosted, WordPress.com). Add DbSiteType enum and mapped_site_id field to reference type-specific tables. Changes: - Add DbSiteType enum with SelfHosted and WordPressCom variants - Implement ToSql/FromSql for DbSiteType using i64 representation - Add site_type and mapped_site_id fields to DbSite struct - Update all DbSite construction in tests and fixtures - Add TODO comments in repository code for fetching site data from sites table
Relocate site-related types to db_types for better organization. Changes: - Create db_types/db_site.rs module with DbSite and DbSiteType - Re-export types from lib.rs for backward compatibility - Update module exports in db_types.rs
Move type-related code from mappings module to db_types for better organization. The mappings name was misleading since we're not doing mappings anymore. Changes: - Move `mappings/term_relationships.rs` to `db_types/db_term_relationship.rs` - Move `mappings/helpers.rs` to `db_types/helpers.rs` - Create `db_types/row_ext.rs` with `ColumnIndex` and `RowExt` traits - Update all imports throughout the codebase - Remove old `mappings` module
Use `db_site_id: RowId` instead of `site: DbSite` in database wrapper types. This avoids unnecessary joins since we already know the site context when querying. Changes: - Replace `site: DbSite` with `db_site_id: RowId` in DbAnyPostWithEditContext, DbAnyPostWithViewContext, DbAnyPostWithEmbedContext - Replace `site: DbSite` with `db_site_id: RowId` in DbTermRelationship - Rename column enum variants from `SiteId` to `DbSiteId` for clarity - Update all construction and test assertions to use `db_site_id` - Improve comment clarity for db_site_id field in DbTermRelationship
Create database structure for storing self-hosted WordPress sites with URL and API root. Changes: - Add `DbSelfHostedSite` struct in `db_types/self_hosted_site.rs` - Create `0006-create-self-hosted-sites-table.sql` migration - Modify `sites` table to add `site_type` and `mapped_site_id` columns - Update test fixtures to properly insert into both tables - Add migration to `MIGRATION_QUERIES` array
Add repository layer for upserting and querying self-hosted WordPress sites. Changes: - Create `SiteRepository` in `repository/sites.rs` - Add table name constants for `self_hosted_sites` and `sites` - Implement `upsert_self_hosted_site` with RETURNING optimization - Implement `select_self_hosted_site` to query by DbSite reference - Implement `select_self_hosted_site_by_url` to query by URL - Add module export in `repository/mod.rs`
Create a separate domain type for self-hosted sites that doesn't include database metadata. This improves the API design and makes it easier to add more fields in the future. Changes: - Add `SelfHostedSite` type with `url` and `api_root` fields - Update `upsert_self_hosted_site` to take `&SelfHostedSite` instead of individual parameters - Add `to_self_hosted_site()` method on `DbSelfHostedSite` for conversion
Update test fixtures to use `SiteRepository` for creating sites instead of manually constructing `DbSite` instances. This ensures test sites are properly inserted into the database and uses real test credentials from `integration_test_credentials`. Changes: - Add `integration_test_credentials` as dev dependency - Modify `test_db()` to return both `Connection` and `DbSite` - Update `test_ctx()` to use actual `DbSite` from repository - Change `create_test_site()` signature to accept `&SelfHostedSite` parameter - Update all test call sites to create `SelfHostedSite` instances - Use real test credentials in default test site creation
The `upsert_self_hosted_site` method had a data consistency bug where updating an existing site URL would create orphaned entries in the sites table. This occurred because only the self_hosted_sites table used upsert logic while the sites table always inserted. Changes: - Add unique constraint on `sites(site_type, mapped_site_id)` in migration 0001 - Update `upsert_self_hosted_site` to use ON CONFLICT clause for sites table - Ensures referential integrity between sites and self_hosted_sites tables
Implemented test suite covering all SiteRepository methods with both positive and negative test cases. Tests use a simple fixture approach without TestContext to avoid pollution from default site creation. Changes: - Rename column from `id` to `rowid` in self_hosted_sites table for consistency with posts tables - Add `Debug`, `Clone`, `PartialEq`, `Eq` derives to `DbSelfHostedSite` for testability - Add `count_all_sites()` method to `SiteRepository` (for testing) - Add schema validation test for `SelfHostedSiteColumn` enum - Add tests for `upsert_self_hosted_site` (insert and update behavior) - Add test verifying the duplicate sites bug fix using `count_all_sites()` method - Add tests for `select_self_hosted_site` (by DbSite reference) - Add tests for `select_self_hosted_site_by_url` - Add edge case tests (non-existent sites, wrong site types) - Add test for multiple sites coexisting - Tests use simple `test_conn` fixture instead of `TestContext` - Tests rely on rowid equality to prove update behavior (no unnecessary counts) All tests pass (67 total, 10 new for SiteRepository).
For consistency with the struct naming pattern where `DbSelfHostedSite` is the database representation, the column enum should be `DbSelfHostedSiteColumn`. Changes: - Rename `SelfHostedSiteColumn` to `DbSelfHostedSiteColumn` - Update all references in `SiteRepository` - Update test name to match new enum name
These types are only used internally by repositories and tests, not
exposed in any public API methods. Keeping them internal provides
better encapsulation.
Changes:
- Remove `pub use db_types::db_site::{DbSite, DbSiteType}` from lib.rs
- Update all modules to import directly from `crate::db_types::db_site`
- Update test modules that were using `crate::DbSiteType` to use the direct import
All tests pass (67 total).
Multi-site tests were verbose, requiring explicit URL construction for each test site. This helper uses an internal atomic counter to generate unique URLs automatically. Changes: - Add `create_random_test_site()` helper function - Uses `RANDOM_TEST_SITE_COUNTER` static atomic to generate unique URLs - Update `posts_multi_site_tests.rs` to use new helper - Update `term_relationships_multi_site_tests.rs` to use new helper - Reduces test site creation from 6 lines to 1 line All tests pass (67 total).
Added core `delete_site()` method that handles deletion across both the sites table and type-specific tables (self_hosted_sites). Also added a convenience wrapper `delete_self_hosted_site_by_url()` for common use cases. The implementation ensures proper cleanup since foreign key constraints cannot be used with polymorphic site references. Panics with a clear message if attempting to delete WordPress.com sites (not yet implemented). Changes: - Add `delete_site()` method that deletes from both tables based on site type - Add `delete_self_hosted_site_by_url()` convenience wrapper - Add `count_all_self_hosted_sites()` for testing - Panic with clear message for unimplemented WordPress.com site deletion - Add 5 comprehensive tests covering: - Deletion from both tables (using repository methods, no raw SQL) - Return value for non-existent sites - Deletion isolation between sites - URL-based deletion - Error cases All tests pass (72 total, 5 new for delete functionality).
The `sites` table has been renamed to `db_sites` to match the pattern established by `DbSite` and other database types. This makes the naming more consistent and clear that it's a database-level polymorphic table. Changes: - Rename `sites` table to `db_sites` in migration 0001 - Update unique index name to `idx_db_sites_unique_site_type_and_mapped_site_id` - Rename `SITES_TABLE` constant to `DB_SITES_TABLE` - Update all foreign key references from `sites(id)` to `db_sites(id)` in: - posts_edit_context table - posts_view_context table - posts_embed_context table - term_relationships table - Update all comments to reference `db_sites table` instead of `sites table` - Rename `count_all_sites()` method to `count_all_db_sites()` - Update all test references to use new naming All tests pass (72 total).
Replace field-by-field comparisons with direct struct equality checks in site repository tests, leveraging the PartialEq implementations on DbSite and DbSelfHostedSite. Changes: - Use assert_eq! on structs instead of comparing individual fields - Reduces test verbosity while maintaining same validation coverage
Standardize error handling across all tests in sites.rs by replacing unwrap() calls with descriptive expect() messages. This improves debugging by providing clear context when tests fail. Changes: - Replace unwrap() with expect() in test_conn fixture (2 calls) - Replace unwrap() with expect() in all test functions (21 calls) - Add descriptive error messages for database operations - Total: 23 unwrap() calls eliminated
Ensure upsert and delete operations maintain data consistency between the polymorphic sites table and type-specific tables by wrapping them in atomic transactions. Changes: - Wrap `upsert_self_hosted_site()` in transaction to prevent orphaned entries - Wrap `delete_site()` in transaction to ensure proper cleanup of both tables - Scope SQL statements in blocks to ensure they drop before `tx.commit()` - Update method signatures to use `TransactionManager` instead of `QueryExecutor` - Update test helpers to accept `&mut Connection` for transactional operations - Add comment explaining `DO UPDATE SET` requirement for `RETURNING` clause
8d970ad to
2179407
Compare
jkmassel
approved these changes
Nov 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Implements a repository pattern for managing WordPress sites in the mobile cache database, supporting both self-hosted and WordPress.com sites through a polymorphic table design.
Key Additions
Database Schema
sites→db_sitesfor clarity (migrations 0001-0005)db_sitestable structure:site_type- Enum discriminator (SelfHosted = 0, WordPressCom = 1)mapped_site_id- References type-specific table rows(site_type, mapped_site_id)prevents duplicatesself_hosted_sitestable:url- Unique site URLapi_root- WordPress REST API endpointSiteRepository Implementation
CRUD Operations (all atomic via transactions):
upsert_self_hosted_site()- Create or update self-hosted siteselect_self_hosted_site()- Retrieve byDbSitereferenceselect_self_hosted_site_by_url()- Retrieve by URLdelete_site()- Remove from both tables atomicallydelete_self_hosted_site_by_url()- Convenience wrapperDesign patterns:
RETURNINGclause for efficient ID retrievalType System
DbSite- Database site record with polymorphic referenceDbSiteType- Enum for site type discriminationSelfHostedSite- Domain model for creating/updating sitesDbSelfHostedSite- Database model including rowidTest Infrastructure
create_random_test_site()- Auto-generates unique test sites via atomic counterTestContextto use real test credentials fromintegration_test_credentialscrateArchitecture Rationale
The polymorphic design allows the
db_sitestable to reference different site type tables based onsite_type, enabling clean separation between self-hosted and WordPress.com site data while maintaining a unified site identifier across the cache system.